-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/cljs devtools support #11
base: master
Are you sure you want to change the base?
Conversation
I'm not thrilled with the duplication here but I see this could be of use to someone. This needs a little more thought before I'd feel comfortable merging something like this. A quick scan seems to reveal the primary difference between the
If we can do both of these I see no reason why the |
@noprompt Your point 1 could be handled via a simple parameter to the tracer. Then there's no need to verify existence of clj-devtools (sounds like hard work for not much advantage). The programmer can turn this feature on or off, via the parameter to the tracer. Which just leaves point 2 -- which would be based off the proposed parameter. |
This commit day8@a6f4f28 for cljs-devtools we have to
I produced a version that does both but the code wasn't really better for it I think. I can definitely revert to that if you like, however, detecting cljs-devtools may be a bit too magical and explicit calling is probably better. |
As far as I know, the clj-devtools are still experimental and Chromium is still a requirement to even use the features it exposes. This already makes me hesitant to consider this patch. It's also why this library is protocol based.
@mike-thompson-day8 Parameterization seems like a good place to start but what other parameters might be needed in the future? |
This PR adds a custom tracer that works well with cljs-devtools. As you see I have a couple of commits where I tried to refactor
default-tracer
but it ended out as worse code.